Repository navigation
Conversation
Add __reduce__ for Action, Result, BasicAuth, FlightDescriptor, Ticket, Location, FlightEndpoint, SchemaResult, and FlightInfo so they round-trip through pickle (via existing serialize/deserialize, or URI for Location).
|
|
lidavidm
left a comment
lidavidm
left a comment
There was a problem hiding this comment.
Full Arrow/Flight builds were not run locally (no Flight-enabled pyarrow on this machine); relying on CI for the Python Flight job.
CI is a scarce resource, especially with the volume of PRs nowadays. The agent should be more than capable of building and testing; I'm not really willing to review if it hasn't done that much.
ump45nose
left a comment
ump45nose
left a comment
There was a problem hiding this comment.
@lidavidm Thanks for the pushback — fair point on CI load.
I built Arrow C++ with ARROW_FLIGHT=ON (bundled gRPC/protobuf/absl/Boost) and a Flight-enabled pyarrow against that install prefix, then ran the pickle coverage locally:
pytest pyarrow/tests/test_flight.py -k pickle→test_pickle_flight_typespassed- also re-ran the related serialize / Location / FlightEndpoint / FlightInfo cases (4 passed)
PR description updated to drop the “relying on CI” note. Happy to run a broader Flight subset if you want a specific list.
(AI-assisted reply.)
|
|
Rationale for this change
Fixes #35150. Flight value types such as
FlightEndpointcannot be pickled because they do not implement__reduce__, which breaks use cases that move Flight metadata across process boundaries (for example Ray).What changes are included in this PR?
Add
__reduce__for the Flight value types that already expose wire-formatserialize/deserialize(Action,Result,BasicAuth,FlightDescriptor,Ticket,FlightEndpoint,SchemaResult,FlightInfo), reducing through those helpers.Locationhas no protobuf serialize helper, so it reduces via its URI.Are these changes tested?
Added
test_pickle_flight_typesnext to the existing serialize round-trip coverage inpython/pyarrow/tests/test_flight.py.Locally built Arrow C++ 27.0.0-SNAPSHOT with
ARROW_FLIGHT=ON(bundled gRPC/protobuf/absl/Boost) and a Flight-enabled pyarrow against that prefix, then ran:Are there any user-facing changes?
No breaking API changes. Pickling Flight value types that previously failed now works.
Was AI used for this PR?
In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.
PR code and description written by:
Reviewed before submission by: